Skip to content

Conversation

@DukeFerdinand
Copy link
Owner

@gibsorya did some excellent work, merging in to upstream :)

@DukeFerdinand DukeFerdinand requested a review from Copilot May 8, 2025 16:52
@DukeFerdinand DukeFerdinand self-assigned this May 8, 2025
@DukeFerdinand DukeFerdinand added the enhancement New feature or request label May 8, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces an asynchronous client module to support non-blocking HTTP requests for the Sanity client and updates the Cargo.toml accordingly.

  • Added a new async_client module with configuration and helper functions
  • Introduced asynchronous implementations for fetching and processing JSON data
  • Updated versioning and added the tokio dependency

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/lib.rs Added declaration for the new async_client module
src/async_client/mod.rs Created module structure and re-exporting of config and helpers
src/async_client/helpers.rs Added async helper to process JSON responses from reqwest
src/async_client/config.rs Added configuration, URL building, and async query execution functions
Cargo.toml Updated package version and added the tokio dependency


impl Query {
pub async fn execute(&self) -> Result<Value, Box<dyn std::error::Error>> {
let url = format!("{} {}", &self.base_url, &self.query.as_ref().unwrap());
Copy link

Copilot AI May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concatenating the base URL and query with a space may lead to an improperly formatted URL. Consider using proper query parameter formatting (e.g., using '?' or '&' as appropriate) and ensuring the query is correctly appended.

Copilot uses AI. Check for mistakes.
pub async fn get_json(
reqwest_res: reqwest::Response,
) -> Result<Value, Box<dyn std::error::Error>> {
let data: Value = serde_json::from_str(&reqwest_res.text().await?)?;
Copy link

Copilot AI May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Instead of converting the response text to JSON manually, it might be clearer and more efficient to use reqwest's built-in JSON deserialization with reqwest_res.json().await.

Suggested change
let data: Value = serde_json::from_str(&reqwest_res.text().await?)?;
let data: Value = reqwest_res.json().await?;

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants